refine stage-run identity and run lifecycle#71
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| _DEFAULT_REGISTRY = RunRegistry() | ||
|
|
||
|
|
||
| def resolve_run(app_id: str | None, *, sign_outputs: bool = False) -> RunContext: | ||
| """Resolve or create the run context for the provided application identifier.""" | ||
|
|
||
| normalised_app = _normalise_app_id(app_id) | ||
| run_id = _dt.datetime.utcnow().strftime("%Y%m%d-%H%M%S") | ||
| root = ARTEFACTS_ROOT | ||
| run_dir = root / normalised_app / run_id | ||
| _prepare_directories(run_dir) | ||
| return RunContext(app_id=normalised_app, run_id=run_id, root=root, sign_outputs=sign_outputs) | ||
|
|
||
|
|
||
| def _normalise_app_id(app_id: str | None) -> str: | ||
| if not app_id: | ||
| return "APP-UNKNOWN" | ||
| candidate = app_id.strip() or "APP-UNKNOWN" | ||
| safe = [ch if ch.isalnum() or ch in ("-", "_") else "-" for ch in candidate] | ||
| return "".join(safe) | ||
|
|
||
|
|
||
| def _json_dumps(data: Mapping[str, Any] | Iterable[Any]) -> str: | ||
| return json.dumps(data, indent=2, sort_keys=True, ensure_ascii=False) | ||
| return _DEFAULT_REGISTRY.create_run(app_id, sign_outputs=sign_outputs) | ||
|
|
||
|
|
||
| def reopen_run(app_id: str | None, run_id: str, *, sign_outputs: bool = False) -> RunContext: | ||
| """Return a run context for an existing run identifier.""" | ||
|
|
||
| normalised_app = _normalise_app_id(app_id) | ||
| root = ARTEFACTS_ROOT | ||
| run_dir = root / normalised_app / run_id | ||
| if not run_dir.exists(): | ||
| raise FileNotFoundError(run_dir) | ||
| _prepare_directories(run_dir) | ||
| return RunContext(app_id=normalised_app, run_id=run_id, root=root, sign_outputs=sign_outputs) | ||
| return _DEFAULT_REGISTRY.reopen_run(app_id, run_id, sign_outputs=sign_outputs) |
There was a problem hiding this comment.
Preserve RunContext helpers for default registry callers
The refactor moved save_input, write_output, etc., from RunContext onto RunRegistry, but the public helpers (resolve_run, reopen_run) still return only a RunContext and never expose the registry instance. Any existing code that previously did ctx = resolve_run(...); ctx.save_input(...) (e.g. tests/test_run_registry.py) now raises AttributeError because the returned context has no way to persist inputs or outputs. Since _DEFAULT_REGISTRY remains private, the module no longer provides any public API to save artefacts when using the convenience functions, effectively breaking all current callers. Either reintroduce these methods on RunContext or expose wrapper functions so the default registry remains usable.
Useful? React with 👍 / 👎.
|
Closing as part of PR consolidation. Useful changes have been cherry-picked into PR #240. |
…b66dc Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68e84c7b1884832998354d353635e374